Add role to reflection with llm #2527
Conversation
… be able to pass the role for the summarizing prompt
… be able to pass the role for the summarizing prompt, minor docstring adjustments
|
Added a test on test_groupchat. |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 10404662 | Triggered | Generic CLI Secret | eff19ac | .github/workflows/dotnet-release.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | e973ac3 | .github/workflows/dotnet-release.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
https://github.com/microsoft/autogen/actions/runs/8881467046/job/24383882709?pr=2527 https://github.com/microsoft/autogen/actions/runs/8881467028/job/24383887017?pr=2527 |
|
The changes are fine. Please follow @sonichi 's comments. Thanks. |
Head branch was pushed to by a user without write access
|
Fixed both comments. |
Co-authored-by: Chi Wang <wang.chi@microsoft.com>
|
The code formatting error can be fixed with pre-commit. |
|
I installed pre-commit, but since I am not using WSL, I had to uninstall to be able to commit. |
No idea what this file was about
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2527 +/- ##
===========================================
- Coverage 33.11% 22.84% -10.27%
===========================================
Files 86 86
Lines 9108 9243 +135
Branches 1938 2131 +193
===========================================
- Hits 3016 2112 -904
- Misses 5837 6888 +1051
+ Partials 255 243 -12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Please fix the CI error on formatting. You can install pre-commit so this will be checked before commit. https://microsoft.github.io/autogen/docs/contributor-guide/pre-commit |
Head branch was pushed to by a user without write access
|
Not sure how or why, but seems the issue is with the test_groupchat file, which got an update in main that commented all functions and created this issue. This merge from main seems to be the cause this issue was present in this branch: b23b4ba |
* Added 'role' as a summary_args and to the reflection_with_llm flow to be able to pass the role for the summarizing prompt * Added 'role' as a summary_args and to the reflection_with_llm flow to be able to pass the role for the summarizing prompt, minor docstring adjustments * Added test for summary prompt role assignment * Fixed docstrings and mocked llm-config in the test * Update autogen/agentchat/conversable_agent.py Co-authored-by: Chi Wang <wang.chi@microsoft.com> * ran pre-commit * ran pre-commit2 * fixed old arg name * Delete dasdaasd No idea what this file was about * Fixed incorrect merge update on test_groupchat --------- Co-authored-by: Chi Wang <wang.chi@microsoft.com> Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
* Added 'role' as a summary_args and to the reflection_with_llm flow to be able to pass the role for the summarizing prompt * Added 'role' as a summary_args and to the reflection_with_llm flow to be able to pass the role for the summarizing prompt, minor docstring adjustments * Added test for summary prompt role assignment * Fixed docstrings and mocked llm-config in the test * Update autogen/agentchat/conversable_agent.py Co-authored-by: Chi Wang <wang.chi@microsoft.com> * ran pre-commit * ran pre-commit2 * fixed old arg name * Delete dasdaasd No idea what this file was about * Fixed incorrect merge update on test_groupchat --------- Co-authored-by: Chi Wang <wang.chi@microsoft.com> Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
Use 'role' in summary_args to use as the role string of the prompt message used to summarize.
Why are these changes needed?
When creating a GroupChat summary, the prompt 'role' is hardcoded as "system". Idea is to enable the initiate_chat() flow to define this by declaring it in system_args. Issue is that there is already a 'role' argument that is passed:
(From the docstrings)
However, I don't see anywhere in the flow that this value is retrieved or used for anything. I'm not entirely sure what the intention was (potentially to distinguish function calls from conversations, but the flow makes that decision irrespective of the 'role' value), so we can either (1) repurpuse this value to use as the role string to pass in the message dict or (2) add a different argument (called 'summary_role' or something) to avoid potential issues.
Given that, to the best of my knowledge, the old 'role' value wasn't being used, I think the best approach is 1.
With these changes, the argument can still be used to distinguish between function and assistant, but now has a 'default' value of 'system', and is used as the string value for the 'role' attribute in the summary prompt message.
Related issue number
Closes #2492
#2644
Checks